zephyr: imgtool: sim: support multiple keys (closes #2700)#2701
zephyr: imgtool: sim: support multiple keys (closes #2700)#2701JPHutchins wants to merge 1 commit into
Conversation
|
Local test runs Click to expand$ cd scripts && /home/jp/repos/mcuboot/scratch/imgtool-venv/bin/pytest -v
===================================== test session starts =====================================
platform linux -- Python 3.12.3, pytest-9.0.3, pluggy-1.6.0 -- /home/jp/repos/mcuboot/scratch/imgtool-venv/bin/python3
cachedir: .pytest_cache
rootdir: /home/jp/repos/mcuboot/scripts
collected 195 items
tests/keys/test_ecdsa.py::EcKeyGeneration::test_emit PASSED [ 0%]
tests/keys/test_ecdsa.py::EcKeyGeneration::test_emit_pub PASSED [ 1%]
tests/keys/test_ecdsa.py::EcKeyGeneration::test_keygen PASSED [ 1%]
tests/keys/test_ecdsa.py::EcKeyGeneration::test_sig PASSED [ 2%]
tests/keys/test_ed25519.py::Ed25519KeyGeneration::test_emit PASSED [ 2%]
tests/keys/test_ed25519.py::Ed25519KeyGeneration::test_emit_pub PASSED [ 3%]
tests/keys/test_ed25519.py::Ed25519KeyGeneration::test_keygen PASSED [ 3%]
tests/keys/test_ed25519.py::Ed25519KeyGeneration::test_sig PASSED [ 4%]
tests/keys/test_rsa.py::KeyGeneration::test_emit PASSED [ 4%]
tests/keys/test_rsa.py::KeyGeneration::test_emit_pub PASSED [ 5%]
tests/keys/test_rsa.py::KeyGeneration::test_keygen PASSED [ 5%]
tests/keys/test_rsa.py::KeyGeneration::test_sig PASSED [ 6%]
tests/test_commands.py::test_new_command PASSED [ 6%]
tests/test_commands.py::test_help PASSED [ 7%]
tests/test_commands.py::test_version PASSED [ 7%]
tests/test_commands.py::test_unknown PASSED [ 8%]
tests/test_commands.py::test_cmd_help[create] PASSED [ 8%]
tests/test_commands.py::test_cmd_help[dumpinfo] PASSED [ 9%]
tests/test_commands.py::test_cmd_help[getpriv] PASSED [ 9%]
tests/test_commands.py::test_cmd_help[getpub] PASSED [ 10%]
tests/test_commands.py::test_cmd_help[getpubhash] PASSED [ 10%]
tests/test_commands.py::test_cmd_help[keygen] PASSED [ 11%]
tests/test_commands.py::test_cmd_help[sign] PASSED [ 11%]
tests/test_commands.py::test_cmd_help[verify] PASSED [ 12%]
tests/test_commands.py::test_cmd_help[version] PASSED [ 12%]
tests/test_commands.py::test_cmd_dif_help[create-create] PASSED [ 13%]
tests/test_commands.py::test_cmd_dif_help[create-dumpinfo] PASSED [ 13%]
tests/test_commands.py::test_cmd_dif_help[create-getpriv] PASSED [ 14%]
tests/test_commands.py::test_cmd_dif_help[create-getpub] PASSED [ 14%]
tests/test_commands.py::test_cmd_dif_help[create-getpubhash] PASSED [ 15%]
tests/test_commands.py::test_cmd_dif_help[create-keygen] PASSED [ 15%]
tests/test_commands.py::test_cmd_dif_help[create-sign] PASSED [ 16%]
tests/test_commands.py::test_cmd_dif_help[create-verify] PASSED [ 16%]
tests/test_commands.py::test_cmd_dif_help[create-version] PASSED [ 17%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-create] PASSED [ 17%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-dumpinfo] PASSED [ 18%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-getpriv] PASSED [ 18%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-getpub] PASSED [ 19%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-getpubhash] PASSED [ 20%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-keygen] PASSED [ 20%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-sign] PASSED [ 21%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-verify] PASSED [ 21%]
tests/test_commands.py::test_cmd_dif_help[dumpinfo-version] PASSED [ 22%]
tests/test_commands.py::test_cmd_dif_help[getpriv-create] PASSED [ 22%]
tests/test_commands.py::test_cmd_dif_help[getpriv-dumpinfo] PASSED [ 23%]
tests/test_commands.py::test_cmd_dif_help[getpriv-getpriv] PASSED [ 23%]
tests/test_commands.py::test_cmd_dif_help[getpriv-getpub] PASSED [ 24%]
tests/test_commands.py::test_cmd_dif_help[getpriv-getpubhash] PASSED [ 24%]
tests/test_commands.py::test_cmd_dif_help[getpriv-keygen] PASSED [ 25%]
tests/test_commands.py::test_cmd_dif_help[getpriv-sign] PASSED [ 25%]
tests/test_commands.py::test_cmd_dif_help[getpriv-verify] PASSED [ 26%]
tests/test_commands.py::test_cmd_dif_help[getpriv-version] PASSED [ 26%]
tests/test_commands.py::test_cmd_dif_help[getpub-create] PASSED [ 27%]
tests/test_commands.py::test_cmd_dif_help[getpub-dumpinfo] PASSED [ 27%]
tests/test_commands.py::test_cmd_dif_help[getpub-getpriv] PASSED [ 28%]
tests/test_commands.py::test_cmd_dif_help[getpub-getpub] PASSED [ 28%]
tests/test_commands.py::test_cmd_dif_help[getpub-getpubhash] PASSED [ 29%]
tests/test_commands.py::test_cmd_dif_help[getpub-keygen] PASSED [ 29%]
tests/test_commands.py::test_cmd_dif_help[getpub-sign] PASSED [ 30%]
tests/test_commands.py::test_cmd_dif_help[getpub-verify] PASSED [ 30%]
tests/test_commands.py::test_cmd_dif_help[getpub-version] PASSED [ 31%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-create] PASSED [ 31%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-dumpinfo] PASSED [ 32%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-getpriv] PASSED [ 32%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-getpub] PASSED [ 33%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-getpubhash] PASSED [ 33%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-keygen] PASSED [ 34%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-sign] PASSED [ 34%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-verify] PASSED [ 35%]
tests/test_commands.py::test_cmd_dif_help[getpubhash-version] PASSED [ 35%]
tests/test_commands.py::test_cmd_dif_help[keygen-create] PASSED [ 36%]
tests/test_commands.py::test_cmd_dif_help[keygen-dumpinfo] PASSED [ 36%]
tests/test_commands.py::test_cmd_dif_help[keygen-getpriv] PASSED [ 37%]
tests/test_commands.py::test_cmd_dif_help[keygen-getpub] PASSED [ 37%]
tests/test_commands.py::test_cmd_dif_help[keygen-getpubhash] PASSED [ 38%]
tests/test_commands.py::test_cmd_dif_help[keygen-keygen] PASSED [ 38%]
tests/test_commands.py::test_cmd_dif_help[keygen-sign] PASSED [ 39%]
tests/test_commands.py::test_cmd_dif_help[keygen-verify] PASSED [ 40%]
tests/test_commands.py::test_cmd_dif_help[keygen-version] PASSED [ 40%]
tests/test_commands.py::test_cmd_dif_help[sign-create] PASSED [ 41%]
tests/test_commands.py::test_cmd_dif_help[sign-dumpinfo] PASSED [ 41%]
tests/test_commands.py::test_cmd_dif_help[sign-getpriv] PASSED [ 42%]
tests/test_commands.py::test_cmd_dif_help[sign-getpub] PASSED [ 42%]
tests/test_commands.py::test_cmd_dif_help[sign-getpubhash] PASSED [ 43%]
tests/test_commands.py::test_cmd_dif_help[sign-keygen] PASSED [ 43%]
tests/test_commands.py::test_cmd_dif_help[sign-sign] PASSED [ 44%]
tests/test_commands.py::test_cmd_dif_help[sign-verify] PASSED [ 44%]
tests/test_commands.py::test_cmd_dif_help[sign-version] PASSED [ 45%]
tests/test_commands.py::test_cmd_dif_help[verify-create] PASSED [ 45%]
tests/test_commands.py::test_cmd_dif_help[verify-dumpinfo] PASSED [ 46%]
tests/test_commands.py::test_cmd_dif_help[verify-getpriv] PASSED [ 46%]
tests/test_commands.py::test_cmd_dif_help[verify-getpub] PASSED [ 47%]
tests/test_commands.py::test_cmd_dif_help[verify-getpubhash] PASSED [ 47%]
tests/test_commands.py::test_cmd_dif_help[verify-keygen] PASSED [ 48%]
tests/test_commands.py::test_cmd_dif_help[verify-sign] PASSED [ 48%]
tests/test_commands.py::test_cmd_dif_help[verify-verify] PASSED [ 49%]
tests/test_commands.py::test_cmd_dif_help[verify-version] PASSED [ 49%]
tests/test_commands.py::test_cmd_dif_help[version-create] PASSED [ 50%]
tests/test_commands.py::test_cmd_dif_help[version-dumpinfo] PASSED [ 50%]
tests/test_commands.py::test_cmd_dif_help[version-getpriv] PASSED [ 51%]
tests/test_commands.py::test_cmd_dif_help[version-getpub] PASSED [ 51%]
tests/test_commands.py::test_cmd_dif_help[version-getpubhash] PASSED [ 52%]
tests/test_commands.py::test_cmd_dif_help[version-keygen] PASSED [ 52%]
tests/test_commands.py::test_cmd_dif_help[version-sign] PASSED [ 53%]
tests/test_commands.py::test_cmd_dif_help[version-verify] PASSED [ 53%]
tests/test_commands.py::test_cmd_dif_help[version-version] PASSED [ 54%]
tests/test_compression.py::test_lzma2_compression[lzma2-True] PASSED [ 54%]
tests/test_compression.py::test_lzma2_compression[disabled-False] PASSED [ 55%]
tests/test_keys.py::test_keygen[rsa-2048] PASSED [ 55%]
tests/test_keys.py::test_keygen[rsa-3072] PASSED [ 56%]
tests/test_keys.py::test_keygen[ecdsa-p256] PASSED [ 56%]
tests/test_keys.py::test_keygen[ecdsa-p384] PASSED [ 57%]
tests/test_keys.py::test_keygen[ed25519] PASSED [ 57%]
tests/test_keys.py::test_keygen[x25519] PASSED [ 58%]
tests/test_keys.py::test_keygen_type[rsa-2048] PASSED [ 58%]
tests/test_keys.py::test_keygen_type[rsa-3072] PASSED [ 59%]
tests/test_keys.py::test_keygen_type[ecdsa-p256] PASSED [ 60%]
tests/test_keys.py::test_keygen_type[ecdsa-p384] PASSED [ 60%]
tests/test_keys.py::test_keygen_type[ed25519] PASSED [ 61%]
tests/test_keys.py::test_keygen_type[x25519] PASSED [ 61%]
tests/test_keys.py::test_getpriv[openssl-rsa-2048] PASSED [ 62%]
tests/test_keys.py::test_getpriv[openssl-rsa-3072] PASSED [ 62%]
tests/test_keys.py::test_getpriv[openssl-ecdsa-p256] PASSED [ 63%]
tests/test_keys.py::test_getpriv[openssl-ecdsa-p384] PASSED [ 63%]
tests/test_keys.py::test_getpriv[openssl-ed25519] XFAIL [ 64%]
tests/test_keys.py::test_getpriv[openssl-x25519] XFAIL [ 64%]
tests/test_keys.py::test_getpriv[pkcs8-rsa-2048] XFAIL [ 65%]
tests/test_keys.py::test_getpriv[pkcs8-rsa-3072] XFAIL [ 65%]
tests/test_keys.py::test_getpriv[pkcs8-ecdsa-p256] PASSED [ 66%]
tests/test_keys.py::test_getpriv[pkcs8-ecdsa-p384] PASSED [ 66%]
tests/test_keys.py::test_getpriv[pkcs8-ed25519] XFAIL [ 67%]
tests/test_keys.py::test_getpriv[pkcs8-x25519] PASSED [ 67%]
tests/test_keys.py::test_getpub[lang-c-rsa-2048] PASSED [ 68%]
tests/test_keys.py::test_getpub[lang-c-rsa-3072] PASSED [ 68%]
tests/test_keys.py::test_getpub[lang-c-ecdsa-p256] PASSED [ 69%]
tests/test_keys.py::test_getpub[lang-c-ecdsa-p384] PASSED [ 69%]
tests/test_keys.py::test_getpub[lang-c-ed25519] PASSED [ 70%]
tests/test_keys.py::test_getpub[lang-c-x25519] PASSED [ 70%]
tests/test_keys.py::test_getpub[lang-rust-rsa-2048] PASSED [ 71%]
tests/test_keys.py::test_getpub[lang-rust-rsa-3072] PASSED [ 71%]
tests/test_keys.py::test_getpub[lang-rust-ecdsa-p256] PASSED [ 72%]
tests/test_keys.py::test_getpub[lang-rust-ecdsa-p384] PASSED [ 72%]
tests/test_keys.py::test_getpub[lang-rust-ed25519] PASSED [ 73%]
tests/test_keys.py::test_getpub[lang-rust-x25519] PASSED [ 73%]
tests/test_keys.py::test_getpub[pem-rsa-2048] PASSED [ 74%]
tests/test_keys.py::test_getpub[pem-rsa-3072] PASSED [ 74%]
tests/test_keys.py::test_getpub[pem-ecdsa-p256] PASSED [ 75%]
tests/test_keys.py::test_getpub[pem-ecdsa-p384] PASSED [ 75%]
tests/test_keys.py::test_getpub[pem-ed25519] XFAIL [ 76%]
tests/test_keys.py::test_getpub[pem-x25519] PASSED [ 76%]
tests/test_keys.py::test_getpub[raw-rsa-2048] PASSED [ 77%]
tests/test_keys.py::test_getpub[raw-rsa-3072] PASSED [ 77%]
tests/test_keys.py::test_getpub[raw-ecdsa-p256] PASSED [ 78%]
tests/test_keys.py::test_getpub[raw-ecdsa-p384] PASSED [ 78%]
tests/test_keys.py::test_getpub[raw-ed25519] PASSED [ 79%]
tests/test_keys.py::test_getpub[raw-x25519] PASSED [ 80%]
tests/test_keys.py::test_getpubhash[lang-c-rsa-2048] PASSED [ 80%]
tests/test_keys.py::test_getpubhash[lang-c-rsa-3072] PASSED [ 81%]
tests/test_keys.py::test_getpubhash[lang-c-ecdsa-p256] PASSED [ 81%]
tests/test_keys.py::test_getpubhash[lang-c-ecdsa-p384] PASSED [ 82%]
tests/test_keys.py::test_getpubhash[lang-c-ed25519] PASSED [ 82%]
tests/test_keys.py::test_getpubhash[lang-c-x25519] PASSED [ 83%]
tests/test_keys.py::test_getpubhash[raw-rsa-2048] PASSED [ 83%]
tests/test_keys.py::test_getpubhash[raw-rsa-3072] PASSED [ 84%]
tests/test_keys.py::test_getpubhash[raw-ecdsa-p256] PASSED [ 84%]
tests/test_keys.py::test_getpubhash[raw-ecdsa-p384] PASSED [ 85%]
tests/test_keys.py::test_getpubhash[raw-ed25519] PASSED [ 85%]
tests/test_keys.py::test_getpubhash[raw-x25519] PASSED [ 86%]
tests/test_keys.py::test_getpub_name_suffix_c[rsa-2048] PASSED [ 86%]
tests/test_keys.py::test_getpub_name_suffix_c[rsa-3072] PASSED [ 87%]
tests/test_keys.py::test_getpub_name_suffix_c[ecdsa-p256] PASSED [ 87%]
tests/test_keys.py::test_getpub_name_suffix_c[ecdsa-p384] PASSED [ 88%]
tests/test_keys.py::test_getpub_name_suffix_c[ed25519] PASSED [ 88%]
tests/test_keys.py::test_getpub_name_suffix_c[x25519] PASSED [ 89%]
tests/test_keys.py::test_getpub_name_suffix_rust[rsa-2048] PASSED [ 89%]
tests/test_keys.py::test_getpub_name_suffix_rust[rsa-3072] PASSED [ 90%]
tests/test_keys.py::test_getpub_name_suffix_rust[ecdsa-p256] PASSED [ 90%]
tests/test_keys.py::test_getpub_name_suffix_rust[ecdsa-p384] PASSED [ 91%]
tests/test_keys.py::test_getpub_name_suffix_rust[ed25519] PASSED [ 91%]
tests/test_keys.py::test_getpub_name_suffix_rust[x25519] PASSED [ 92%]
tests/test_keys.py::test_getpub_name_suffix_rejected[pem] PASSED [ 92%]
tests/test_keys.py::test_getpub_name_suffix_rejected[raw] PASSED [ 93%]
tests/test_keys.py::test_getpubhash_name_suffix_c[rsa-2048] PASSED [ 93%]
tests/test_keys.py::test_getpubhash_name_suffix_c[rsa-3072] PASSED [ 94%]
tests/test_keys.py::test_getpubhash_name_suffix_c[ecdsa-p256] PASSED [ 94%]
tests/test_keys.py::test_getpubhash_name_suffix_c[ecdsa-p384] PASSED [ 95%]
tests/test_keys.py::test_getpubhash_name_suffix_c[ed25519] PASSED [ 95%]
tests/test_keys.py::test_getpubhash_name_suffix_c[x25519] PASSED [ 96%]
tests/test_keys.py::test_getpubhash_name_suffix_rejects_raw PASSED [ 96%]
tests/test_keys.py::test_sign_verify[rsa-2048] PASSED [ 97%]
tests/test_keys.py::test_sign_verify[rsa-3072] PASSED [ 97%]
tests/test_keys.py::test_sign_verify[ecdsa-p256] PASSED [ 98%]
tests/test_keys.py::test_sign_verify[ecdsa-p384] PASSED [ 98%]
tests/test_keys.py::test_sign_verify[ed25519] PASSED [ 99%]
tests/test_keys.py::test_sign_verify[x25519] XFAIL [100%]
=============================== 188 passed, 7 xfailed in 7.95s ================================
$ cd sim && cargo test --features sig-ed25519 --test core
warning: use of deprecated function `rand::thread_rng`: Renamed to `rng`
--> sim/src/image.rs:1707:29
|
1707 | let mut rng = rand::thread_rng();
| ^^^^^^^^^^
|
= note: `#[warn(deprecated)]` on by default
warning: use of deprecated method `rand::Rng::gen_range`: Renamed to `random_range`
--> sim/src/image.rs:1711:37
|
1711 | let reset_counter = rng.gen_range(1 ..= remaining_ops / 2);
| ^^^^^^^^^
warning: `bootsim` (lib) generated 2 warnings
Finished `test` profile [optimized + debuginfo] target(s) in 0.06s
Running tests/core.rs (/home/jp/repos/mcuboot/target/debug/deps/core-658591a82fff9de3)
running 28 tests
test dependency_combos ... ok
test bootstrap ... ok
test hw_prot_failed_security_cnt_check ... ok
test hw_prot_missing_security_cnt ... ok
test direct_xip_first ... ok
test oversized_bootstrap ... ok
test multi_key::signed_primary_key_boots ... ok
test norevert_newimage ... ok
test oversized_secondary_slot ... ok
test ram_load_corrupt_higher_version_image ... ok
test ram_load_missing_header_flag ... ok
test multi_key::signed_unknown_key_rejected ... ok
test ram_load_failed_validation ... ok
test ram_load_from_flash ... ok
test bad_secondary_slot ... ok
test multi_key::single_key_build_rejects_secondary_key_image ... ok
test ram_load_first ... ok
test downgrade_prevention ... ok
test ram_load_split ... ok
test ram_load_out_of_bounds ... ok
test secondary_trailer_leftover ... ok
test perm_with_random_fails ... ok
test norevert ... ok
test status_write_fails_complete ... ok
test status_write_fails_with_reset ... ok
test basic_revert ... ok
test perm_with_fails has been running for over 60 seconds
test revert_with_fails has been running for over 60 seconds
test perm_with_fails ... ok
test revert_with_fails ... ok
test result: ok. 28 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 284.80s
$ cd sim && cargo test --features 'sig-ed25519 sig-second-key' --test core
Compiling bootsim v0.1.0 (/home/jp/repos/mcuboot/sim)
warning: use of deprecated function `rand::thread_rng`: Renamed to `rng`
--> sim/src/image.rs:1707:29
|
1707 | let mut rng = rand::thread_rng();
| ^^^^^^^^^^
|
= note: `#[warn(deprecated)]` on by default
warning: use of deprecated method `rand::Rng::gen_range`: Renamed to `random_range`
--> sim/src/image.rs:1711:37
|
1711 | let reset_counter = rng.gen_range(1 ..= remaining_ops / 2);
| ^^^^^^^^^
warning: `bootsim` (lib) generated 2 warnings
Finished `test` profile [optimized + debuginfo] target(s) in 4.54s
Running tests/core.rs (/home/jp/repos/mcuboot/target/debug/deps/core-fc4e87a8e8bb9e24)
running 28 tests
test dependency_combos ... ok
test oversized_bootstrap ... ok
test hw_prot_failed_security_cnt_check ... ok
test hw_prot_missing_security_cnt ... ok
test bootstrap ... ok
test norevert_newimage ... ok
test direct_xip_first ... ok
test multi_key::signed_secondary_key_boots ... ok
test oversized_secondary_slot ... ok
test multi_key::signed_unknown_key_rejected ... ok
test ram_load_corrupt_higher_version_image ... ok
test multi_key::signed_primary_key_boots ... ok
test ram_load_failed_validation ... ok
test bad_secondary_slot ... ok
test ram_load_from_flash ... ok
test downgrade_prevention ... ok
test ram_load_out_of_bounds ... ok
test ram_load_missing_header_flag ... ok
test ram_load_first ... ok
test norevert ... ok
test secondary_trailer_leftover ... ok
test ram_load_split ... ok
test perm_with_random_fails ... ok
test status_write_fails_complete ... ok
test status_write_fails_with_reset ... ok
test basic_revert ... ok
test perm_with_fails has been running for over 60 seconds
test revert_with_fails has been running for over 60 seconds
test perm_with_fails ... ok
test revert_with_fails ... ok
test result: ok. 28 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 293.63s |
JPHutchins
force-pushed
the
feature-zephyr-multiple-signing-keys
branch
from
e035433 to
13aca63
Compare
|
the idea here would need @de-nordic to look at it and approve, but do not add another Kconfig for another key, how do you add a 3rd key? a 9th key? 300 keys? Just use multiple files in the existing Kconfig and iterate them as a list in cmake |
No additional Kconfigs, if we are going to go with this then we should rather list keys into the existing I also do not see a reason to have multiple keys, that differ, if they basically open the same lock. You can not tell a difference between them, they do not change behaviour of an app at the MCUboot level. |
Good idea, I think that's achievable. The one wrinkle that might come up during Zephyr integration is selection of the application signing key. With multiple KConfig entries, the primary key can state that it will be used to sign the image. With a single KConfig entry, that help text would instead state that the first key in the list will be used to sign the image, and therefore needs to be a private signing key. See the draft integration here for details: intercreate/zephyr#1 |
Let's discuss at #2700. It is an MCUBoot feature designed to improve security, not weaken it. My Zephyr integration draft demonstrates correct use of public keys. I will take note that I should warn users about use of private keys in the examples and KConfig. I do agree that "increased chance of guessing keys" has no security impact, given their cryptographic attributes. |
If a list is provided then CMake can provide the first item in the list to the images and the full value to MCUboot, the help text can be updated to specify this
I assume what Dominik is meaning is if you generate/store all the keys on the same system then one key getting compromised = all keys getting compromised, and the other that if the key is cracked through bruteforce (assuming that in the future the time required to bruteforce a key is reduced substantially) then bruteforcing one is not much difference to bruteforcing two, and the final point that if a key is cracked by luck (and this has happened in the past to some very big name company's) then having 2 keys where they are both always valid means that you do not add any extra security to the device |
|
@nordicjm @de-nordic Thanks again for taking the time to review! I'm eager to dig into the details here and make the necessary adjustments. It seems like the main concern is that two or more keys that are always valid create more attack surface - from one or more of the keys being leaked or from a lucky or quantum crack - and that it does not have a significant security benefit. This is a fair concern if the keys have equivalent custody and their compromise has equivalent blast radius, but this feature exists precisely to enable the workflow where they do not. Threat ModelThe threat model this feature targets is one of custody, not cryptographic strength. The keys are cryptographically equivalent; they are not operationally equivalent. Asymmetric cryptography lets public and private material live under asymmetric custody, and this feature lets the Zephyr port use that distinction.
The security gain is at the custody layer, not the cryptography layer. This feature lets organizations stop exercising the prod private key for day-to-day engineering work. Every signing event is operational risk, every additional person with access to the prod private key is operational risk, and reducing both is one of the most effective ways to protect a long-lived signing key. Design IntentThe feature is not a new direction for MCUboot, but rather an implementation of an MCUboot capability that Zephyr is unable to use. From docs/signed_images.md:
From docs/readme-zephyr.md:
#2702 is the parallel imgtool change formalizing public-only PEM support for exactly this workflow: same dev/prod custody pattern, applied to the signing tool rather than the bootloader. Discussion
I agree, and it would represent a misuse of this feature, since it exists to enable separate custody. I'll add CMake-time validation that only the first of the list is private, KConfig help text, and docs to make sure teams don't misuse this feature. The Zephyr integration will include a sample app with standard security practice (intercreate/zephyr#1).
Agreed. This feature is not intended to strengthen cryptographic security; it strengthens custody.
True, but as above, this doesn't apply directly since the feature is not claiming to strengthen the cryptographic security. Worth noting that only tightly held developer units would be flashed with a bootloader capable of booting from N keys. Production units should be flashed with a bootloader that trusts only the prod key. A lucky crack of the dev key gives an attacker no path onto a prod unit.
Correct, and they should not differ there. The differentiation is which bootloader binary embeds which public keys, not the verification logic.
The custody of the different keys is not equivalent, and adding a second key does not change the loss probability of the first. Losing a dev private signing key compromises the inventory of non-deployed hardware. Losing the production private signing key is the catastrophic event, and this feature reduces the chance of that happening. I agree that a revocation mechanism would be valuable, but that is a separate feature and not related to updating Zephyr's port to support MCUBoot's existing multi-key verification. SummaryThe feature enables a documented MCUboot capability that the Zephyr port currently can't use. I agree with the cryptographic-strength concerns. That is not the threat this PR addresses. I also agree that the multi-key capability may be misused, but with updates to validation, documentation, and samples, it will be a net gain by providing a way for teams to maintain narrow custody of their production private signing keys. I will get started on the following improvements:
|
JPHutchins
force-pushed
the
feature-zephyr-multiple-signing-keys
branch
2 times, most recently
from
95fb2b6 to
4f2345c
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds multi-signing-key support across the simulator and Zephyr build, plus imgtool enhancements to generate multiple embedded key symbols and validate key material type.
Changes:
- Add simulator support for choosing between primary/secondary/unknown Ed25519 signing keys and introduce a multi-key test matrix.
- Extend Zephyr’s
CONFIG_BOOT_SIGNATURE_KEY_FILEto accept a semicolon-separated list and generate multiple embedded verification keys. - Add imgtool features:
--name-suffixforgetpub/getpubhashsymbol disambiguation and a newkeyinfocommand for build-time key kind enforcement.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| sim/tests/core.rs | Adds multi-key simulator tests gated by signature features |
| sim/src/tlv.rs | Introduces SigningKey selection and multi-key Ed25519 signing support |
| sim/src/lib.rs | Exposes tlv publicly so integration tests can access SigningKey |
| sim/src/image.rs | Threads signing-key selection through image installation/build helpers |
| sim/src/ed25519_pub_key_2-rs.txt | Adds embedded secondary Ed25519 public key bytes for simulator |
| sim/src/ed25519_pub_key_unknown-rs.txt | Adds embedded “unknown” Ed25519 public key bytes for negative tests |
| sim/mcuboot-sys/csupport/keys.c | Adds optional second verification key and fixes key count computation |
| sim/mcuboot-sys/build.rs | Adds sig-second-key feature gating and defines MCUBOOT_SIGN_KEY_2 |
| sim/mcuboot-sys/Cargo.toml | Introduces sig-second-key simulator feature |
| sim/Cargo.toml | Wires sig-second-key feature through to mcuboot-sys |
| scripts/tests/test_keys.py | Adds tests for --name-suffix and new keyinfo command |
| scripts/tests/test_commands.py | Registers keyinfo as a supported CLI command in tests |
| scripts/imgtool/main.py | Implements --name-suffix and adds keyinfo command |
| scripts/imgtool/keys/general.py | Adds name_suffix support to C/Rust symbol emission helpers |
| root-ed25519-unknown.pem | Adds an additional private key used by simulator negative tests |
| root-ed25519-2.pem | Adds a second private key used by simulator multi-key tests |
| root-ed25519-2-pub.pem | Adds the corresponding public-only PEM for documentation/Zephyr use |
| docs/signed_images.md | Documents multi-key usage in Zephyr and custody model rationale |
| docs/readme-zephyr.md | Documents multi-key key-file list behavior and constraints |
| docs/imgtool.md | Documents --name-suffix and keyinfo usage |
| boot/zephyr/sample.yaml | Adds a Zephyr sample test case embedding two signing keys |
| boot/zephyr/keys.c | Implements multi-key bootutil_keys[] via generated suffixed symbols |
| boot/zephyr/Kconfig | Updates help text to describe semicolon-separated key list behavior |
| boot/zephyr/CMakeLists.txt | Generates multiple autogen-pubkey*.c files and enforces public-only keys past the first |
| .github/workflows/sim.yaml | Adds CI matrix entry to run simulator with sig-second-key |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JPHutchins
force-pushed
the
feature-zephyr-multiple-signing-keys
branch
from
4f2345c to
c550c9b
Compare
| endif() | ||
| endif() | ||
|
|
||
| set(mcuboot_sign_key_count_max 8) |
There was a problem hiding this comment.
there should be no reason for a limit, CMake can process all files in the list then provide the number that were supplied to the .c files
There was a problem hiding this comment.
Agreed, and it ended up simplifying the code, fixed now.
| if(mcuboot_key_count GREATER ${mcuboot_sign_key_count_max}) | ||
| message(FATAL_ERROR | ||
| "CONFIG_BOOT_SIGNATURE_KEY_FILE lists ${mcuboot_key_count} keys, but the " | ||
| "Zephyr port is capped at ${mcuboot_sign_key_count_max}. Raise mcuboot_sign_key_count_max " | ||
| "in boot/zephyr/CMakeLists.txt to extend it.") |
| endif() | ||
| else() | ||
| set(name_suffix_arg "--name-suffix" "_${key_index}") | ||
| set(generated_pubkey ${ZEPHYR_BINARY_DIR}/autogen-pubkey${key_index}.c) |
There was a problem hiding this comment.
good to have a - between pubkey and number
There was a problem hiding this comment.
Added
set(generated_pubkey ${ZEPHYR_BINARY_DIR}/autogen-pubkey-${key_index}.c)| set(generated_pubkey ${ZEPHYR_BINARY_DIR}/autogen-pubkey${key_index}.c) | ||
| set(keyinfo_check_command | ||
| COMMAND ${PYTHON_EXECUTABLE} ${MCUBOOT_DIR}/scripts/imgtool.py | ||
| keyinfo --key ${resolved_key_path} --require public |
There was a problem hiding this comment.
CMake indent is 2 spaces
| if(${resolved_key_path} IN_LIST mcuboot_default_signature_files) | ||
| message(WARNING "WARNING: Using default MCUboot signing key file for verification key #${key_index}, this file is for debug use only and is not secure!") | ||
| endif() | ||
| target_compile_definitions(app PRIVATE MCUBOOT_SIGN_KEY_${key_index}=1) |
There was a problem hiding this comment.
not sure why this is needed, you just need the total number to be supplied
There was a problem hiding this comment.
Agreed, it was simpler to not have a max. Fixed now.
| held under separate custody. Embedded private material is | ||
| recoverable from flash, so a public-only first entry is the safer | ||
| default whenever the signing workflow allows. |
There was a problem hiding this comment.
I assume by flash you mean on a build system, should probably be updated to mention storage drives since this sort of sounds like the device you are flashing
There was a problem hiding this comment.
Good catch - yes, this is about the host build system, not the target.
JPHutchins
force-pushed
the
feature-zephyr-multiple-signing-keys
branch
from
c550c9b to
2e08cd8
Compare
| endif() | ||
| endif() | ||
|
|
||
| set(mcuboot_sign_key_count 1) |
There was a problem hiding this comment.
this is not correct in the case of having a bootloader that uses hashes only without signatures, can probably be gated in the bit below since if you are using hashes only then you don't need a define to know how many keys you have since you don't and shouldn't have any
There was a problem hiding this comment.
Fixed in latest push.
JPHutchins
force-pushed
the
feature-zephyr-multiple-signing-keys
branch
from
2e08cd8 to
e40157f
Compare
nordicjm
left a comment
nordicjm
left a comment
There was a problem hiding this comment.
Zephyr parts look good, just needs comma change
| if(${signing_key_file} IN_LIST mcuboot_default_signature_files) | ||
| message(WARNING "WARNING: Using default MCUboot signing key file, this file is for debug use only and is not secure!") | ||
| endif() | ||
| set(mcuboot_key_files "${CONFIG_BOOT_SIGNATURE_KEY_FILE}") |
There was a problem hiding this comment.
noticed an issue earlier whilst testing this with sysbuild with ; and it seems the simple way to resolve that (which is used elsewhere) is use commas to separate keys, then have string(REPLACE "," ";" mcuboot_key_files "${CONFIG_BOOT_SIGNATURE_KEY_FILE}") here to convert commmas to semi-colons
There was a problem hiding this comment.
Fixed, updating the Zephyr integration next
There was a problem hiding this comment.
Updated the Zephyr integration, it did simplify things, thanks!
| The key file will be parsed by imgtool's getpub command and a .c source | ||
| with the public key information will be written in a format expected by | ||
| MCUboot. | ||
| Path to a signing/verification key PEM, or a semicolon-separated |
There was a problem hiding this comment.
and update prompt/comment
zephyr: support multiple signing keys CONFIG_BOOT_SIGNATURE_KEY_FILE accepts a semicolon-separated list of PEMs. The first entry may be a keypair or public-only PEM — only the public bytes are ever embedded in the bootloader binary; subsequent entries must be public-only and are validated at CMake configure time. This enables the prod/dev custody model: a development bootloader can boot both production-signed and development-signed images, while production bootloaders embed only the production public key. imgtool: add --name-suffix to getpub and getpubhash Emits suffixed C/Rust symbol names so more than one key of the same signature type can be embedded without collisions. imgtool: add keyinfo subcommand Reports whether a PEM is a keypair (private) or public-only. Used by the Zephyr build to enforce that verification-only entries past the first are public-only PEMs. sim: tests for multiple ed25519 keys Add the sig-second-key feature and multi_key test scenarios covering single-key and dual-key builds. Documentation updated in docs/readme-zephyr.md, docs/signed_images.md, and docs/imgtool.md. Closes mcu-tools#2700. Signed-off-by: JP Hutchins <jp@intercreate.io>
JPHutchins
force-pushed
the
feature-zephyr-multiple-signing-keys
branch
from
e40157f to
bbd2e95
Compare
Closes #2700.
Summary
Extend the Zephyr port to optionally embed a second signing-verification key in the bootloader. When
CONFIG_BOOT_SIGNATURE_KEY_FILE_2is set, the bootloader accepts images signed by either the primary or the secondary key. When it is empty (the default), the compiled output is unchanged relative to current upstream — no new symbols, no new array entries, no runtime cost.See the linked issue for motivation, user demand, and the underlying documented-but-unimplemented design intent.
Testing
Verified:
imgtoolunit tests (scripts/tests/test_keys.py): pass, including the new--name-suffixpositive and negative cases across all supported key types.sig-ed25519alone and withsig-ed25519,sig-second-key: the four newmulti_keyscenarios pass under both configurations, with the#[cfg]-gated variants applying correctly.Not yet verified:
boot/zephyr/keys.ccode path that production builds use, so I expect parity, but that's inference, not a measurement. Happy to perform hardware validation before merge if that's the expectation, or as a follow-up if simulator + imgtool-test coverage is considered sufficient for a gated, off-by-default feature. Please advise.CI expected to exercise:
Simworkflow — new feature combinations.Build Zephyr samples with Twister— existing coverage unaffected; the new option defaults off.imgtoolworkflow — new unit tests.Design questions for reviewers
Three points where I made a choice but should be considered open:
1. Symbol-renaming mechanism. I added
--name-suffixtoimgtool getpub/getpubhash. The alternative was a post-processing rename script inside the Zephyr CMake build. I preferred theimgtoolroute because it's reusable across ports (Mynewt, Espressif, sim), unit-testable, and keeps the generated sources clean. If you'd rather keepimgtool's CLI surface smaller, I can move the rename logic into the build system — let me know.2.
autogen-pubkey2.cvsautogen-pubkey2.h. The existing mechanism for the primary key emitsbuild/zephyr/autogen-pubkey.hand#includes it fromboot/zephyr/keys.c. This PR emitsautogen-pubkey2.cand links it as a separate source file instead. I chose this because it keepskeys.cstructurally unchanged (just an addedexternblock) and avoids a conditional#includeof a file that may not exist. If you prefer symmetry with the primary-key mechanism (i.e. a.h#included conditionally), I'll switch — small change, called out here so it's not a surprise.3. Scope of
sig-second-keyin the simulator. Currently wired forsig-ed25519only. Extending to RSA and ECDSA insim/mcuboot-sys/csupport/keys.cis mechanical (anotherroot_pub_der_2for each type). Happy to do it in this PR if you'd rather see full matrix coverage before merge, or as a follow-up if you'd rather land the Zephyr-port change first.4. Kconfig naming. I went with
BOOT_SIGNATURE_KEY_FILE_2. The alternative is a list-styleBOOT_SIGNATURE_KEY_FILEStaking whitespace-separated paths. In practice every multi-key fleet I've seen is "prod + dev", so two is the right starting point, and_Nleaves room to grow. Push back if you'd rather pick the list form from the start.